Skip to content

Consistently (don't) split long source file lines#5967

Draft
Josue-Crz wants to merge 4 commits intoopenwall:bleeding-jumbofrom
Josue-Crz:bleeding-jumbo-forkb
Draft

Consistently (don't) split long source file lines#5967
Josue-Crz wants to merge 4 commits intoopenwall:bleeding-jumbofrom
Josue-Crz:bleeding-jumbo-forkb

Conversation

@Josue-Crz
Copy link
Copy Markdown

@Josue-Crz Josue-Crz commented Mar 26, 2026

Issue #3896

I built upon magnumripper's previous merged PR #3908 in which I soft split each part of the test vector divided by the delimiter ('$'). Enforcing a soft split per section of the vector test string within a limit of 80 characters per line, increasing readability toward meaning to each delimiter value of the long string vector test.

As well, I propose a max_line_length of 80 within future header files in .editorconfig to increase readability across long test vectors.

Testing my changes to openbsd-softraid:

~/Documents/Github_Repos/john-JosueFork/run$ ./john --test --format=openbsd-softraid

Output:

Will run 16 OpenMP threads
Benchmarking: OpenBSD-SoftRAID [PBKDF2-SHA1 256/256 AVX2 8x]... (16xOMP) DONE
Speed for cost 1 (kdf) of 1, cost 2 (iteration count) of 8192
Raw: 9170 c/s real, 588 c/s virtual

@Josue-Crz
Copy link
Copy Markdown
Author

@solardiz Could you please look at this when you have the chance? I just want to know I'm in the right direction when tackling this issue.

As well, I'm familiar Magnumripper's approach in PR #3908 with removing escape characters in the long test vectors, should I combine Magnumripper's approach to the issue of removing escape characters with the styling I have in my current PR draft?

@magnumripper
Copy link
Copy Markdown
Member

Thanks! I have some objections but don't let that discourage you.

I soft split each part of the test vector divided by the delimiter ('$'). Enforcing a soft split per section of the vector test string within a limit of 80 characters per line

I can see some merit in splitting at the delimiter but that's not in #3896 and we'd want to discuss it first. I'm not sure I like it. Would we also do so with short test vectors that otherwise need no splitting?

Also, we definitely do not want the splits as short as 80 characters, way longer:

#3896 was originally only about replacing backslash continuation with multiple string constants, like in #3908, and split (or not) consistently. Later (in same issue) we decided to split lines at "no lower, or not much lower, than Jim's split in openbsdsoftraid_variable_code.h was. Maybe 1000 or 2000 or 4000,". The original split was ~1480, so I think we should go for ~2000.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants